Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --disable-tests option #475

Merged
merged 1 commit into from
Mar 23, 2023
Merged

Add --disable-tests option #475

merged 1 commit into from
Mar 23, 2023

Conversation

ffontaine
Copy link
Contributor

Add --disable-tests to allow the user to disable tests. As a side-effect, this will avoid the following build failure when check is found:

libstat_wrapper.c:11:10: fatal error: gnu/lib-names.h: No such file or directory
   11 | #include <gnu/lib-names.h>
      |          ^~~~~~~~~~~~~~~~~

This build failure is raised since version 2.0.5 and 78df90b

Fixes:

Signed-off-by: Fabrice Fontaine [email protected]

Add --disable-tests to allow the user to disable tests. As a
side-effect, this will avoid the following build failure when check is
found:

libstat_wrapper.c:11:10: fatal error: gnu/lib-names.h: No such file or directory
   11 | #include <gnu/lib-names.h>
      |          ^~~~~~~~~~~~~~~~~

This build failure is raised since version 2.0.5 and
78df90b

Fixes:
 - http://autobuild.buildroot.org/results/450cfc36d4fd6dc71c138bec45f05b5a2d92a08d

Signed-off-by: Fabrice Fontaine <[email protected]>
@knet-ci-bot
Copy link

Can one of the admins verify this patch?

@fabbione
Copy link
Member

fabbione commented Nov 4, 2022

ok to test

@fabbione
Copy link
Member

fabbione commented Nov 4, 2022

Add --disable-tests to allow the user to disable tests. As a side-effect, this will avoid the following build failure when check is found:

I have no objections to add --disable-tests, tho:

libstat_wrapper.c:11:10: fatal error: gnu/lib-names.h: No such file or directory
   11 | #include <gnu/lib-names.h>
      |          ^~~~~~~~~~~~~~~~~

This build failure is raised since version 2.0.5 and 78df90b

this part I don´t understand. We run CI on each build and we have never seen this problem before.

What distribution / container / platform are you building on? Is the version of check too old?

@ffontaine
Copy link
Contributor Author

ffontaine commented Nov 4, 2022

The build failure is raised by buildroot autobuilders.
check is in version 0.15.2 but check doesn't provide gnu/lib-names.h.
This file is glibc-specific, it will raise the above build failure with uclibc(-ng) and musl.

@chrissie-c
Copy link
Contributor

I think it would be better just to disable that test. It's a good thing to test libqb against other libcs where possible.

@ffontaine
Copy link
Contributor Author

Indeed, you could disable or remove this test however it still makes sense to disable all tests to save build time and disk space.

@fabbione
Copy link
Member

fabbione commented Nov 4, 2022

The build failure is raised by buildroot autobuilders. check is in version 0.15.2 but check doesn't provide gnu/lib-names.h. This file is glibc-specific, it will raise the above build failure with uclibc(-ng) and musl.

Apologize for my complete ignorance, but what is buildroot.org?

+1 on disabling the test when gnu/lib-names.h is not available.

Regardless of the libc is in use, test suite should be built and executed. Build only doesn´t help much for the final results. Shaving 3/4 seconds to build is meaningless if the result doesn´t work on the libc in use.

@tpetazzoni
Copy link

Why is gnu/lib-names.h even included? I don't see anything from this header file that gets used in libstat_wrapper.c. Removing the gnu/lib-names.h solves the build issue.

@chrissie-c chrissie-c merged commit 1a32a60 into ClusterLabs:main Mar 23, 2023
@chrissie-c
Copy link
Contributor

I'll remove it. I suspect it was needed for some earlier OS version, but without that it's passed all the CI tests, so it's obviously fine without!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants